Responsive header refactor - #14
Merged
Merged
Conversation
…tion - Part 2 - Working as expected
…tion - Part 3 - Adding indicator to expanding nav sub menu items
…tion - Part 4 - Lock indicator to details summary when toggling
# Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
… and document pointer-events requirement Merges the duplicate .overflow-sub-nav-indicator-hovered rules into one. Adds a comment explaining why pointer-events: none is critical — without it the indicator (z-index: 1) swallows mouse events, clears hoveredChildKey, and causes a flicker loop on hover.
…dden width feedback loop
- Move all component styling from default.vue into ResponsiveHeader.vue and
NavigationItems.vue via CSS custom property token system
- Token defaults live at var(--token, fallback) at usage sites — NOT declared on
the component root element, avoiding source-order cascade conflicts with consumers
- Add public token comment blocks documenting all available override tokens
- Rename consumer scope class from 'your-scope-class' to 'site-header-nav'
- default.vue now contains only page layout CSS plus a clean .site-header-nav
token-override block (~360 lines removed)
- Add color/active-indicator tokens: --responsive-nav-decorator-indicator-color,
--overflow-nav-decorator-indicator-color, --responsive-header-color
- Fix visually-hidden item width feedback loop (7995px runaway):
- Root cause: overflow: hidden sets flex min-width to 0 but does not cap
max-width; each hide→measure cycle read the inflated (content + margin) width
- Fix 1: inline-size default changed from auto to 100% so the wrapper is
properly constrained within its grid column
- Fix 2: inline-size: var(--_main-navigation-item-width) added to .visually-hidden
state, pinning the <li> to its originally-measured width so offsetWidth always
returns the correct value regardless of shifted inner content
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add --responsive-nav-decorator-hovered-indicator-color token (main nav) - Add --overflow-nav-decorator-hovered-indicator-color token (overflow nav) - Use CSS :has(li.is-hovered) to switch the active indicator bar to the hover colour when the bar has tracked to a hovered item, keeping active and hover visually distinct - Update .overflow-sub-nav-indicator-hovered border to use hover indicator token - default.vue: active indicator = white (--slate-00), hover indicator = green (--green-08) - Tokens fall back gracefully — consumers only need to set the indicator color if they want hover and active to share the same colour Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace invalid native CSS &__child concatenation with explicit descendant selectors across three demo pages (esbuild was silently converting them to :is(__child) which matched nothing): - scroll-reveal-image.vue: &__info → & .sri-demo-row__info, &__heading / &__fields → & .demo-controls__heading/fields - site-navigation-demo.vue: &--wide, &__text, &__img, &__body all lifted to top-level rules; media query hoisted out of modifier block - page-hero-highlights.vue: &__heading, &__toggles, &__selects replaced with explicit .demo-controls__* selectors Other changes: - simple-grid.vue: replace GridCore with LayoutGridByWidth - Add .claude/skills/css-nesting-conventions.md — documents the &__child pitfall, why it silently fails, and correct patterns - Add .claude/skills/pull-request-description.md skill - CLAUDE.md: add pitfall #11 (Sass-style BEM nesting in native CSS)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors ResponsiveHeader and its overflow menu (NavigationItems) to use a token-driven hover/active indicator system aligned with TabNavigation, and fixes native CSS nesting pitfalls (Sass-style &__child / &--mod concatenation) across several demo pages.
Changes:
- Refactored
ResponsiveHeader+ overflow navigation to drive hovered/active indicators via CSS tokens and anchor-positioned indicator elements. - Fixed invalid native CSS nesting selectors on demo/layout pages and documented the pitfall in
.claude/skills+CLAUDE.md. - Updated the simple grid demo to use
LayoutGridByWidthinstead ofGridCore.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Claude.md |
Adds a documented pitfall about Sass-style BEM nesting in native CSS. |
app/pages/ui/simple-grid.vue |
Switches demo grid implementation to LayoutGridByWidth and adjusts styles. |
app/pages/ui/scroll-reveal-image.vue |
Replaces invalid native nesting (&__...) with valid descendant selectors. |
app/pages/page-hero-highlights.vue |
Replaces invalid native nesting (&__...) with valid class selectors. |
app/layouts/site-navigation-demo.vue |
Fixes invalid nesting/modifier patterns by expanding to valid CSS selectors. |
app/layouts/default.vue |
Updates ResponsiveHeader usage and replaces bespoke styling with token overrides. |
app/components/responsive-header/ResponsiveHeader.vue |
Adds token API + hover/active indicator elements and revises geometry/measurement flow. |
app/components/responsive-header/NavigationItems.vue |
Adds hover/active classes, indicator elements, and panel-animation gating for indicators. |
.vscode/settings.json |
Tweaks workspace settings (removes TS SDK comment/setting, disables AI co-author). |
.claude/skills/pull-request-description.md |
Adds a new skill for generating PR descriptions as fenced markdown. |
.claude/skills/index.md |
Registers the new skills in the skills index. |
.claude/skills/css-nesting-conventions.md |
Documents native CSS nesting rules and the &__child failure mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…er race - Add @focusout handler on main-navigation that clears hoveredItemKey when focus moves outside the nav entirely (checked via relatedTarget); previously, keyboard Tab past the last nav item left an item stuck in is-hovered with the indicator still visible - Guard the ResizeObserver async callback with an isMeasuring/pendingMeasure mutex using the run-latest do/while pattern; the two-pass geometry path awaits multiple ticks and toggles isOverflowVisibleForMeasurement — concurrent observer fires could race on that state and corrupt mainNavigationState Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors
ResponsiveHeaderandNavigationItemsto bring their hover/active indicator system in line withTabNavigation. Indicators are now driven by a public CSS token API and correctly track hover and active state for both top-level items and sub-nav children. Also fixes a CSS nesting bug across three demo pages where Sass-style&__childselectors were silently producing no output.Changes
NavigationItems.vue— addedis-hovered/is-activeclasses driven byhoveredItemKey,hoveredChildKey(mouseenter/mouseleave), andisActiveNavItem()(route comparison); added dedicated indicator elements (overflow-nav-indicator-hovered,overflow-nav-indicator-active,overflow-sub-nav-indicator-hovered);isPanelAnimatingflag prevents indicator jump during<details>open/close transitionResponsiveHeader.vue— migrated all CSS to the public token system (documented inline); added separate--overflow-nav-decorator-hovered-indicator-colorand active colour tokens; fixed visually-hidden width feedback loop; fixed nav item underflowing the toggle buttondefault.vue— trimmed down following the ResponsiveHeader refactorsite-navigation-demo.vue,page-hero-highlights.vue,scroll-reveal-image.vue— replaced&__child/&--modifierBEM concatenation with valid native CSS (& .block__childor top-level rules); these selectors were being converted to:is(__child)by esbuild and matching nothingsimple-grid.vue— switched fromGridCoretoLayoutGridByWidth.claude/skills/css-nesting-conventions.md— new skill documenting the native CSS nesting pitfall and correct patternsCLAUDE.md— added pitfall Tooltips #11 for Sass-style BEM nestingTesting
npm run build) with no new warningsNotes
The
isPanelAnimatingflag is a workaround for a visual jump that occurs when a<details>sub-menu opens/closes while an indicator is tracking position. It suppresses indicator transitions for the duration ofDETAILS_ANIMATION_DURATION.